r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

r? @sfackler

Logistically I want #64152 to land before this, and then I want to leverage that to ensure that the standard library only has one piece of code for printing backtraces instead of both here and for panics.

(in case anyone's wondering why it's duplicated here)

Thanks for doing this @alexcrichton!

This is nice functionality, but because it is added directly to std::error::Error, it blocks #64017
which would have succeeded in moving Error to the alloc crate, or at least, that seems to be why it was closed?

Can we consider doing this differently so that these PR's are still compatible? It would really help rust portability if std::error::Error could be the one recommendable way to represent errors. Ideally it can eventually be in core, and moving to alloc is big progress. If it is forever going to be an std-only thing then it will make it hard to be used in embedded, and then libraries that want to be embedded-friendly won't be able to use std::error::Error.

I think one good way might be to make a Backtrace trait, and make Error::backtrace() return Option<&dyn Backtrace>. There's some overhead for the virtual dispatch, but your backtrace object contains dynamic allocations anyways, so it's probably negligible. This kind of overhead is similar to fn source(&self) -> Option<&dyn Error + 'static> anyways. It looks that Backtrace interface is extremely simple.

There are a few other ways I thought about, you probably can think of more than I can.
Interested what you think.

@cbeck88 There are solutions to someday making the Error trait available to no_std applications. Two possible solutions to the problem of backtraces, which don't require changing this API at all, are discussed toward the end of the closed PR.

I've updated with the various comments here. @cbeck88 I don't have anything to add over what @withoutboats already mentioned.

@bors r+

📌 Commit abe20c5a2fc489c1f2b5d41dbe43421c87e3078e has been approved by sfackler

@bors: r-

just gonna be sure to sequence this behind #64152, but I'll carry over the r+ after that rebase. Thanks for reviewing @sfackler!

@withoutboats @alexcrichton thanks, makes sense

☔ The latest upstream changes (presumably #64281) made this pull request unmergeable. Please resolve the merge conflicts.

@bors: r=sfackler

📌 Commit abe20c5a2fc489c1f2b5d41dbe43421c87e3078e has been approved by sfackler

@bors: r=sfackler

📌 Commit 34662c6 has been approved by sfackler

⌛ Testing commit 34662c6 with merge bba531caa5a6e8291c3f37dd3c44700f436e6cd5...

💥 Test timed out

@bors: retry

Looks like a normal timeout, but let's see if it's affected by this..

⌛ Testing commit 34662c6 with merge fe6d05a...

☀️ Test successful - checks-azure
Approved by: sfackler
Pushing fe6d05a to master...

📣 Toolstate changed by #64154!

Tested on commit fe6d05a.
Direct link to PR: #64154

💔 rls on linux: test-pass → test-fail (cc @Xanewok, @rust-lang/infra).

This PR broke xargo, or at least the Xargo configuration used by Miri:

error[E0432]: unresolved import `crate::sys_common::backtrace`
  --> /home/travis/build/RalfJung/miri-test-libstd/rust-src-patched/src/libstd/backtrace.rs:98:24
   |
98 | use crate::sys_common::backtrace::{output_filename, lock};
   |                        ^^^^^^^^^ could not find `backtrace` in `sys_common`

@alexcrichton is it possible that libstd no longer compiles with the default features?

EDIT: Ah, this was already reported at #64410.